Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.x] Store ObjectID instead of pointer for KinematicCollision owner #90669

Merged
merged 1 commit into from
Apr 16, 2024

Conversation

timothyqiu
Copy link
Member

3.x version of #90668


Reproduction scripts (run with godot --no-window -s /path/to/script.gd, no window not necessary):

move_and_collide.gd
extends SceneTree

var player
var saved_collision

func _initialize() -> void:
    _build_scene()


func _iteration(delta: float) -> bool:
    match Engine.get_physics_frames():
        0:
            # Move and save collision info.
            var collision = player.move_and_collide(Vector3.DOWN)
            saved_collision = collision
            print("[Frame 0] Saved collision: ", saved_collision)

        1:
            # Generate another collision info.
            var collision = player.move_and_collide(Vector3.DOWN)
            print("[Frame 1] Collision: ", collision)

        2:
            print("[Frame 2] Deleting player...")
            player.queue_free()
            player = null

        3:
            print("[Frame 3] Trying to access freed shape...")
            print(saved_collision.get_local_shape())
            quit()
    
    return false


func _build_scene() -> void:
    # A sphere on the top of a box.
    _build_player()
    _build_floor()


func _build_player() -> void:
    var shape = SphereShape.new()

    var body = KinematicBody.new()
    body.position = Vector3(0, shape.radius, 0)

    var collision = CollisionShape.new()
    collision.shape = shape
    body.add_child(collision)

    root.add_child(body)
    player = body


func _build_floor() -> void:
    var shape = BoxShape.new()

    var body = StaticBody.new()
    body.position = Vector3(0, -shape.extents.y, 0)

    var collision = CollisionShape.new()
    collision.shape = shape
    body.add_child(collision)

    root.add_child(body)
get_slide_collision.gd
extends SceneTree

var player
var saved_collision

func _initialize() -> void:
    _build_scene()


func _iteration(delta: float) -> bool:
    match Engine.get_physics_frames():
        0:
            # Move and save collision info.
            player.move_and_slide_with_snap(Vector3(1, -1, 0) * delta, Vector3.DOWN)
            var collision = player.get_slide_collision(0)
            saved_collision = collision
            print("[Frame 0] Saved collision: ", saved_collision)

        1:
            # Generate another collision info.
            player.move_and_slide_with_snap(Vector3(1, -1, 0) * delta, Vector3.DOWN)
            var collision = player.get_slide_collision(0)
            print("[Frame 1] Collision: ", collision)

        2:
            print("[Frame 2] Deleting player...")
            player.queue_free()
            player = null

        3:
            print("[Frame 3] Trying to access freed shape...")
            print(saved_collision.get_local_shape())
            quit()

    return false


func _build_scene() -> void:
    # A sphere on the top of a box.
    _build_player()
    _build_floor()


func _build_player() -> void:
    var shape = SphereShape.new()

    var body = KinematicBody.new()
    body.position = Vector3(0, shape.radius, 0)

    var collision = CollisionShape.new()
    collision.shape = shape
    body.add_child(collision)

    root.add_child(body)
    player = body


func _build_floor() -> void:
    var shape = BoxShape.new()

    var body = StaticBody.new()
    body.position = Vector3(0, -shape.extents.y, 0)

    var collision = CollisionShape.new()
    collision.shape = shape
    body.add_child(collision)

    root.add_child(body)

@lawnjelly
Copy link
Member

Very similar to the problem in #88946 (which was storing RIDs rather than raw pointers). 👍

I'll just try and work out if there are any performance implications from this, as there are other options available for performance sensitive parts.

@lawnjelly lawnjelly merged commit db3fe5e into godotengine:3.x Apr 16, 2024
13 checks passed
@lawnjelly
Copy link
Member

Thanks!

@timothyqiu timothyqiu deleted the owner-id-3.x branch April 16, 2024 12:51
@lawnjelly
Copy link
Member

@timothyqiu If you could review #88946 that would be super helpful, as that PR is similar to this one, and getting reviewers is super difficult for 3.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants